-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Conversation
I think we should use a fixed-size array instead of a infinitely growable vec. |
} | ||
let jump = self.stack.pop_back(); | ||
if jump >= U256::from(usize::max_value()) { | ||
return Err(vm::Error::InvalidSub) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need a separate error type for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use errors from JUMPDEST
of course, but I think using separate error types is just more descriptive. It does not hurt anyway!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I meant why not using BadSubDestination
as we do return BadJumpDestination
for verify_jump
in such a case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm yeah you're right!
@@ -91,6 +109,27 @@ impl SharedCache { | |||
jump_dests.shrink_to_fit(); | |||
Arc::new(jump_dests) | |||
} | |||
|
|||
fn find_sub_destinations(code: &[u8]) -> Arc<BitSet> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function is used together with find_jump_destinations
, so I guess it could be merge into one avoiding extra pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah you're right!
impl MallocSizeOf for Dests { | ||
fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { | ||
self.jumpdests.size_of(ops) + self.subdests.size_of(ops) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't it be derived with #[derive(MallocSizeOf)]
?
#[doc = "Jump to a subrountine."] | ||
JUMPSUB = 0xb3, | ||
#[doc = "Begin a subrountine."] | ||
BEGINSUB = 0xb5, | ||
#[doc = "Return from a subrountine."] | ||
RETURNSUB = 0xb7, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://eips.ethereum.org/EIPS/eip-2315
We suggest the following opcodes:
0xb2 BEGINSUB
0xb3 JUMPSUB
0xb7 RETURNSUB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, cc @gcolvin. What are the integer values of those opcodes? The one used in Geth PR and in the spec are different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sorpaas I think we should follow the spec ethereum/go-ethereum#20619 (comment)
This PR will be superseded by a future one by @adria0! |
This is a draft of implementation of EIP-2315 (https://eips.ethereum.org/EIPS/eip-2315) simple subroutine. Code locations can be marked as subroutines, and opcodes are introduced to jump to it and return from it (via a newly-introduced "return stack").
rel https://github.com/openethereum/openethereum/issues/11562